-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Introduce electric-sql & some fixes #686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…PostgreSQL configuration - Added Electric SQL service to docker-compose for real-time data synchronization. - Introduced PostgreSQL configuration for logical replication and performance tuning. - Created scripts for initializing Electric SQL user and electrifying tables. - Implemented notification model and service in the backend. - Developed ElectricProvider and useNotifications hook in the frontend for managing notifications. - Updated environment variables and package dependencies for Electric SQL integration.
- Added initialization script for Electric SQL user in Docker setup. - Updated Electric SQL client to support new PGlite architecture and sync functionality. - Improved notification fetching and syncing logic in useNotifications hook. - Refactored ElectricProvider to handle initialization state and errors more gracefully. - Removed deprecated electric.config.ts file and adjusted package dependencies accordingly.
|
@AnishSarkar22 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by RecurseML
🔍 Review performed on 9655db1..f441c7b
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (16)
• docker-compose.yml
• scripts/docker/electrify-tables.sql
• scripts/docker/init-electric-user.sql
• scripts/docker/init-postgres.sh
• scripts/docker/postgresql.conf
• surfsense_backend/alembic/versions/60_add_notifications_table.py
• surfsense_backend/app/db.py
• surfsense_backend/app/services/notification_service.py
• surfsense_web/.env.example
• surfsense_web/app/layout.tsx
• surfsense_web/components/providers/ElectricProvider.tsx
• surfsense_web/hooks/use-notifications.ts
• surfsense_web/lib/electric/auth.ts
• surfsense_web/lib/electric/client.ts
• surfsense_web/package.json
• surfsense_web/pnpm-lock.yaml
…ic SQL integration - Added notifications table to the database schema with replication support for Electric SQL. - Developed NotificationService to manage indexing notifications, including creation, updates, and status tracking. - Introduced NotificationButton and NotificationPopup components for displaying notifications in the UI. - Enhanced useNotifications hook for real-time notification syncing using PGlite live queries. - Updated package dependencies for Electric SQL and improved error handling in notification processes.
… UI and performance - Removed Badge component from NotificationPopup and replaced it with status icons for better visual representation. - Refactored getStatusBadge to getStatusIcon for clarity and updated icon sizes. - Enhanced useNotifications hook to utilize a ref for initialization tracking, improving performance and preventing unnecessary re-initializations. - Optimized notification fetching logic with improved error handling and real-time updates using Electric SQL. - Adjusted loading state management to ensure UI responsiveness during data fetching.
- Added setup_electric_replication function to handle Electric SQL replication for the notifications table during app startup. - Updated alembic migration script to remove direct SQL commands for replication, now managed in app/db.py. - Refactored indexing functions in search_source_connectors_routes to support new start_date and end_date parameters for improved flexibility. - Enhanced Google Gmail indexing task to utilize new date parameters, ensuring better control over indexing periods.
- Refactored multiple indexing functions in search_source_connectors_routes to utilize a new helper function for handling notifications. - Removed redundant error handling and logging from individual indexing tasks, streamlining the code.
…e updates - Introduced a new notifications table in the database schema to manage user notifications. - Implemented Electric SQL replication setup for the notifications table, ensuring real-time synchronization. - Updated existing database functions to support real-time updates for connectors and documents using Electric SQL. - Refactored UI components to utilize new hooks for fetching connectors and documents, enhancing performance and user experience.
…hod and add notification types - Removed the deprecated create_connector_indexed_notification method from NotificationService. - Introduced new notification types and schemas in notification.types.ts to standardize notification handling. - Updated useNotifications hook to utilize the new notification type definitions.
…services - Introduced a new DocumentProcessingNotificationHandler to manage notifications for document processing stages. - Updated existing notification methods to include detailed progress updates for various stages (queued, parsing, chunking, embedding, storing, completed, failed). - Refactored NotificationService to support the new document processing notification type and metadata schema. - Updated multiple document processing tasks to create and manage notifications throughout the processing lifecycle. - Adjusted UI components to reflect changes in notification types and improve user experience during document uploads and processing.
- Introduced a new custom hook, useIndexingConnectors, to track indexing states of connectors without polling. - Updated ConnectorIndicator and related components to utilize the new indexing state management, enhancing user experience with immediate feedback during indexing. - Removed deprecated log summary references and replaced them with real-time updates from Electric SQL. - Adjusted UI components to reflect changes in indexing status, improving clarity and responsiveness.
- Replaced the previous "Syncing..." text with a more concise "Syncing" in various components to enhance clarity. - Adjusted the UI to maintain consistent styling and improve user experience during indexing operations. - Simplified conditional rendering for indexing status across ConnectorCard, ActiveConnectorsTab, and ConnectorAccountsListView components.
- Updated the handleSubmitConnectForm function to accept an additional parameter for indexing start notification, improving the connector form submission process. - Adjusted tooltip styling across various components for consistency and improved visual clarity. - Simplified conditional rendering in ActiveConnectorsTab and ConnectorAccountsListView to enhance UI responsiveness during indexing operations.
…moved redirects for upload button
- Introduced new notifications API routes for marking notifications as read and marking all as read, with automatic syncing via Electric SQL. - Updated the frontend hook to utilize the new API for marking notifications as read, enhancing the user experience with real-time updates. - Included necessary response models for API interactions.
- Added session refresh for notifications to prevent stale data after rollbacks in multiple document processing tasks. - Wrapped notification update logic in try-except blocks to handle potential failures gracefully and log errors without crashing the process. - Improved error handling for notification updates in various document processing functions, enhancing overall robustness.
- Added an `updated_at` timestamp field to the Notification model for better tracking of notification updates. - Updated the NotificationButton component to change the unread notification badge color for improved visibility. - Adjusted the NotificationPopup component's layout and text handling for better responsiveness and readability.
- Introduced a new notifications table with relevant fields and indexes. - Configured Electric SQL replication for the notifications, search_source_connectors, and documents tables. - Centralized Electric SQL user credentials in the migration script for better management. - Ensured idempotency in the migration process for creating users and publications.
- Created a new notifications table with necessary fields and indexes. - Set up Electric SQL replication for notifications, search_source_connectors, and documents tables. - Implemented idempotent checks for user and publication creation in the migration script.
… `init-electric-user.sh`
…ctric SQL database management - Added cleanup logic for other users' databases on login to ensure data isolation. - Introduced best-effort cleanup on logout to remove stale databases. - Updated Electric client initialization to support user-specific databases. - Refactored hooks to utilize the Electric context for better state management and prevent race conditions. - Enhanced error handling and logging for Electric SQL operations.
- Implemented user-level sync for notifications to reduce duplicate subscriptions and improve memory efficiency. - Separated user-level sync from search-space-level queries for smoother transitions between search spaces. - Updated error handling and logging for sync operations. - Improved cleanup logic for subscriptions to prevent memory leaks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by RecurseML
🔍 Review performed on 0d2a2f8..9ddcd4f
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (50)
• .env.example
• Dockerfile.allinone
• docker-compose.yml
• scripts/docker/entrypoint-allinone.sh
• scripts/docker/init-electric-user.sh
• scripts/docker/init-postgres.sh
• scripts/docker/postgresql.conf
• scripts/docker/supervisor-allinone.conf
• surfsense_backend/alembic/env.py
• surfsense_backend/alembic/versions/66_add_notifications_table_and_electric_replication.py
• surfsense_backend/app/db.py
• surfsense_backend/app/routes/__init__.py
• surfsense_backend/app/routes/notifications_routes.py
• surfsense_backend/app/routes/search_source_connectors_routes.py
• surfsense_backend/app/services/notification_service.py
• surfsense_backend/app/tasks/celery_tasks/connector_tasks.py
• surfsense_backend/app/tasks/celery_tasks/document_tasks.py
• surfsense_backend/app/tasks/connector_indexers/discord_indexer.py
• surfsense_backend/app/tasks/connector_indexers/notion_indexer.py
• surfsense_backend/app/tasks/connector_indexers/slack_indexer.py
• surfsense_backend/app/tasks/connector_indexers/teams_indexer.py
• surfsense_backend/app/tasks/connector_indexers/webcrawler_indexer.py
• surfsense_backend/app/tasks/document_processors/file_processors.py
• surfsense_web/.env.example
• surfsense_web/app/dashboard/[search_space_id]/documents/(manage)/components/DocumentsTableShell.tsx
• surfsense_web/app/dashboard/[search_space_id]/documents/(manage)/components/ProcessingIndicator.tsx
• surfsense_web/app/dashboard/[search_space_id]/documents/(manage)/page.tsx
• surfsense_web/app/dashboard/[search_space_id]/editor/[documentId]/page.tsx
• surfsense_web/app/dashboard/[search_space_id]/team/page.tsx
• surfsense_web/app/layout.tsx
• surfsense_web/components/UserDropdown.tsx
• surfsense_web/components/assistant-ui/attachment.tsx
• surfsense_web/components/assistant-ui/connector-popup.tsx
• surfsense_web/components/assistant-ui/connector-popup/components/connector-card.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/views/connector-connect-view.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/views/connector-edit-view.tsx
• surfsense_web/components/assistant-ui/connector-popup/hooks/use-connector-dialog.ts
• surfsense_web/components/assistant-ui/connector-popup/hooks/use-indexing-connectors.ts
• surfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsx
• surfsense_web/components/assistant-ui/connector-popup/tabs/all-connectors-tab.tsx
• surfsense_web/components/assistant-ui/connector-popup/views/connector-accounts-list-view.tsx
• surfsense_web/components/assistant-ui/document-upload-popup.tsx
• surfsense_web/components/assistant-ui/tooltip-icon-button.tsx
• surfsense_web/components/layout/providers/LayoutDataProvider.tsx
• surfsense_web/components/layout/ui/header/Header.tsx
• surfsense_web/components/notifications/NotificationButton.tsx
• surfsense_web/components/notifications/NotificationPopup.tsx
• surfsense_web/components/providers/ElectricProvider.tsx
• surfsense_web/components/shared/llm-config-form.tsx
• surfsense_web/components/sources/DocumentUploadTab.tsx
⏭️ Files skipped (19)
| Locations |
|---|
surfsense_backend/uv.lock |
surfsense_web/components/tool-ui/generate-podcast.tsx |
surfsense_web/components/ui/tooltip.tsx |
surfsense_web/content/docs/connectors/meta.json |
surfsense_web/content/docs/how-to/electric-sql.mdx |
surfsense_web/content/docs/how-to/meta.json |
surfsense_web/content/docs/meta.json |
surfsense_web/contracts/enums/connector.ts |
surfsense_web/contracts/types/document.types.ts |
surfsense_web/contracts/types/notification.types.ts |
surfsense_web/hooks/use-connectors-electric.ts |
surfsense_web/hooks/use-documents-electric.ts |
surfsense_web/hooks/use-notifications.ts |
surfsense_web/lib/electric/auth.ts |
surfsense_web/lib/electric/client.ts |
surfsense_web/lib/electric/context.ts |
surfsense_web/messages/en.json |
surfsense_web/package.json |
surfsense_web/pnpm-lock.yaml |
…precated status - Refactored `useConnectorStatus` hook to utilize `useCallback` for improved performance. - Updated class name for the deprecated status badge to maintain consistency in styling.
- Removed unused type import from `AllPrivateChatsSidebar` and `AllSharedChatsSidebar`. - Adjusted class names for icons in sidebar components to enhance visual consistency. - Updated `SidebarSection` to remove unnecessary gap classes for cleaner layout. - Changed sync version in `client.ts` to v2 for user-specific database architecture. - Modified search chat placeholder text in `en.json` for clarity.
- Deleted `composer-action.tsx`, `composer.tsx`, and `thread-welcome.tsx` files to streamline the codebase and eliminate unused components. - This cleanup is part of an effort to improve maintainability and reduce complexity in the assistant UI.
- Simplified chat document processing display by removing the book emoji for a cleaner look. - Enhanced the greeting function to prioritize user display names over email for a more personalized experience. - Adjusted the ChatShareButton component by removing unused imports and unnecessary elements for better clarity and performance. - Updated the title in the Electric SQL documentation for conciseness.
…ration - Added NEXT_FRONTEND_URL environment variable to docker-compose.yml for frontend integration. - Updated hero-section.tsx to change the "Get Started" button link from "/login" to "/register". - Enhanced electric-sql documentation with detailed instructions for locating and editing the postgresql.conf file.
…or improved UI - Adjusted styling in ActiveConnectorsTab to enhance visual clarity by modifying the Cable component's class. - Removed unnecessary elements from ChatShareButton to streamline the UI and updated the loading text for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by RecurseML
🔍 Review performed on 9ddcd4f..4281e6f
| Severity | Location | Issue | Delete |
|---|---|---|---|
| surfsense_backend/scripts/docker/entrypoint.sh:18 | Ineffective database readiness check |
✅ Files analyzed, no issues (24)
• .env.example
• docker-compose.yml
• scripts/docker/init-electric-user.sh
• surfsense_backend/app/tasks/chat/stream_new_chat.py
• surfsense_web/components/assistant-ui/composer-action.tsx
• surfsense_web/components/assistant-ui/composer.tsx
• surfsense_web/components/assistant-ui/connector-popup/components/connector-status-badge.tsx
• surfsense_web/components/assistant-ui/connector-popup/hooks/use-connector-status.ts
• surfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsx
• surfsense_web/components/assistant-ui/thread-welcome.tsx
• surfsense_web/components/assistant-ui/thread.tsx
• surfsense_web/components/homepage/hero-section.tsx
• surfsense_web/components/layout/ui/sidebar/AllPrivateChatsSidebar.tsx
• surfsense_web/components/layout/ui/sidebar/AllSharedChatsSidebar.tsx
• surfsense_web/components/layout/ui/sidebar/Sidebar.tsx
• surfsense_web/components/layout/ui/sidebar/SidebarSection.tsx
• surfsense_web/components/new-chat/chat-share-button.tsx
• surfsense_web/components/new-chat/document-mention-picker.tsx
• surfsense_web/content/docs/how-to/electric-sql.mdx
• surfsense_web/content/docs/how-to/meta.json
• surfsense_web/lib/electric/client.ts
• surfsense_web/messages/en.json
• surfsense_web/package.json
• surfsense_web/pnpm-lock.yaml
| echo "Running database migrations..." | ||
| # Wait for database to be ready (max 30 seconds) | ||
| for i in {1..30}; do | ||
| if python -c "from app.db import engine; import asyncio; asyncio.run(engine.dispose())" 2>/dev/null; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The database readiness check is ineffective and will pass even when the database is not actually reachable. The code only imports the engine and calls dispose(), but SQLAlchemy's create_async_engine() is lazy and doesn't establish a connection until it's actually used. The engine.dispose() method will succeed without testing database connectivity because no connection pool exists yet. This means the script will proceed to run Alembic migrations (line 27: alembic upgrade head) before the database is truly ready, causing migration failures. The proper fix would be to execute a simple query like await conn.execute(text('SELECT 1')) to actually test connectivity. This will cause intermittent migration failures in production when PostgreSQL takes longer than expected to start.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
/documents.display_namefor greeting in new chat.Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR introduces Electric SQL integration to enable real-time synchronization of notifications between the backend and frontend. It sets up PostgreSQL logical replication with a new Electric SQL service in Docker, creates a notifications table with proper replication configuration, and implements a client-side sync system using PGlite (in-browser PostgreSQL) to enable offline-first real-time notifications. The changes include database schema updates, Docker infrastructure modifications, and a complete frontend integration with React hooks for consuming synchronized notifications.
⏱️ Estimated Review Time: 1-3 hours
💡 Review Order Suggestion
scripts/docker/postgresql.confscripts/docker/init-electric-user.sqldocker-compose.ymlscripts/docker/init-postgres.shsurfsense_backend/alembic/versions/60_add_notifications_table.pysurfsense_backend/app/db.pysurfsense_backend/app/services/notification_service.pyscripts/docker/electrify-tables.sqlsurfsense_web/lib/electric/client.tssurfsense_web/lib/electric/auth.tssurfsense_web/components/providers/ElectricProvider.tsxsurfsense_web/hooks/use-notifications.tssurfsense_web/app/layout.tsxsurfsense_web/.env.examplesurfsense_web/package.jsonsurfsense_web/pnpm-lock.yaml